[9주차/윤샘] 워크북 제출합니다#59
Conversation
YoungJJun
left a comment
YoungJJun
left a comment
There was a problem hiding this comment.
9주차 피드백
-
주니어에 처음하시는데 선택 미션까지.. 대단해요 윤샘!!
-
CustomOAuthService
@Transactional추가하는거 어떨까요?memberRepository.save()호출하니까 예외발생시 롤백 고려해서요! -
CustomOAuthService
Map<String, Object> attributes = oAuthMember.getAttribute("kakao_account"); Map<String, Object> profile = (Map<String, Object>) attributes.get("profile");
위 코드와 관련해서
kakao_account,profile관련해서 null 체크가 이루어지지 않는것 같아요. (다른 부분에서 체크되었으면 무시하셔도 됩니다.)일반적으로 소셜 로그인시에 각 제공자마다 받을 수 있는 정보가 다르고 생각보다 제공되는게 많이 없습니다.. 특히 사업자 추가해서 비즈니스 앱으로 등록하지 않으면 더욱 더 그랬던 것 같아요. 그리고 사용자가 제공에 대해 동의를 안할수도 있구요.
이러한 경우 NPE 발생할 수 있습니다. 고려해서 null인 경우 처리하는 로직 필요할 것 같아요! 여기서 할 필요는 없을 것 같고 나중에 프젝 진행하실 때 참고하시면 될 것 같아용.
-
MemberReqDTO
password변수명에 오타있습니다! 근데 MemberService에도 동일하게 오타난 채로 통일되서 스웨거에서도 오타난채로 통일되서 문제가 안생긴 것 같아요. 수정하면 좋을 것 같습니다. -
MemberService -로그인
@Transactional붙어있는데 조회만 하는 것 같아요readOnly = true붙이면 좋을 것 같습니다. -
CustomOAuthService - loadUser()
loadUser()는 Spring Security OAuth2 인증 필터가 직접 호출하는 메서드입니다.이 흐름에서 던진 예외는
AuthenticationException(그 하위인OAuth2AuthenticationException포함)일 때만 OAuth2 실패 핸들러로 정상적으로 넘어가요.지금은
MemberException(그냥RuntimeException)을 던지고 있어서, Security가 인증 실패로 인식하지 못하고 필터를 그대로 빠져나가 500 에러로 이어집니다.그래서
OAuth2AuthenticationException으로 감싸서 던져야 할 것 같습니당.
윤샘 이번주도 수고하셨습니다. 시험 잘보세용~ 🍎
chazy-d
left a comment
chazy-d
left a comment
There was a problem hiding this comment.
윤샘 9주차도 정말 수고하셨습니다~~!! 6전공이라 많이 힘들었을텐데 대단하십니다 🥹
시험 끝나고 봬요~
| return Jwts.builder() | ||
| .subject(member.getUsername()) // User 이메일을 Subject로 | ||
| .claim("role", authorities) | ||
| .claim("email", member.getUsername()) // 일반 로그인 | ||
| .claim("memberId", member.getMember().getId()) | ||
| .issuedAt(Date.from(now)) // 언제 발급한지 | ||
| .expiration(Date.from(now.plus(expiration))) // 언제까지 유효한지 | ||
| .signWith(secretKey) // sign할 Key | ||
| .compact(); | ||
| } |
There was a problem hiding this comment.
오! JWT 인증 기준을 email에서 memberId로 바꾼거 좋은데요? email은 변경될 수 있고 내부 식별자로 조회하는 것 좋은 것 같아요! 저도 적용해보겠습니다.
그런데 JWT 인증 복원은 memberId claim을 기준으로 이루어지는 것 같습니다. 그렇다면 email claim은 현재 흐름에서는 사용되지 않는 정보처럼 보였고, 인증 기준을 memberId로 잡는다면 JWT의 subject도 email 대신 memberId로 맞추는 방식도 가능할 것 같습니다!!
근데 윤샘은 db id uuid로 저장하는거 선호하시나요? auto-increment int 방식으로 구현하는 것 선호하시나요?
| // nullable = false 거나 NullPointerException이 발생 가능한 attribute에는 더미 | ||
| public static Member toMember(OAuthDTO dto) { | ||
| return Member.builder() | ||
| .name(dto.getName()) | ||
| .email(dto.getSocialEmail()) | ||
| .password("") | ||
| .gender(Gender.NONE) | ||
| .birth(LocalDate.of(2000, 1, 1)) | ||
| .address("NONE") | ||
| .socialProvider(dto.getSocialProvider()) | ||
| .socialId(dto.getSocialId()) | ||
| .currentPoint(0L) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
윤샘이 해당 부분에 더미를 두었던 이유를 추적해보며 왜 Member 엔티티에서 nullable=false를 이렇게 많이 지정했지? 생각해보았습니다.
이건 약간 OAuth 로그인 성공 = 서비스 가입 완료라는 시선이 잡혀있어서 그런 것 같습니다! (아님말고)
이미지의 와이어프레임처럼 서비스에서 필요한 추가적인 정보를 받아야 할 경우
방법 A. OAuth 성공 시 Member를 먼저 만든다
→ profileCompleted=false 또는 status=PENDING_PROFILE
→ 추가 정보 입력 완료 시 ACTIVE
방법 B. OAuth 성공 시 Member를 아직 만들지 않는다
→ socialUid/email 등을 임시 토큰이나 세션에 담음
→ 추가 정보 입력까지 끝난 뒤 Member 생성
위의 방법등을 사용하게 됩니다.
이렇게 보면 회원가입 상태 모델링 자체에서 비밀번호/생년월일/주소 없이 소셜 회원을 만들 수 있게 하거나, 아예 생성 시점을 뒤로 미루는 방식도 가능할 것 같은데 이 부분은 어떻게 생각하시는지 궁금합니다!
| @Transactional | ||
| public MemberResDTO.Login login(MemberReqDTO.Login request){ | ||
| Member member = memberRepository.findByEmail(request.email()) | ||
| .orElseThrow(() -> new MemberException(MemberErrorCode.LOGIN_FAILED)); | ||
|
|
||
| if(!passwordEncoder.matches(request.paassword(), member.getPassword())){ | ||
| throw new MemberException(MemberErrorCode.LOGIN_FAILED); | ||
| } | ||
|
|
||
| String accessToken = jwtUtil.createAccessToken(new AuthMember(member)); | ||
|
|
||
| return MemberConverter.toLogin(accessToken); | ||
| } |
There was a problem hiding this comment.
로그인 로직에서 먼저 사용자의 이메일을 검증하고, 비밀번호를 해싱해서 기존 값이랑 검증하는 흐름이 명확하게 잘 보입니다! 또 이메일이 존재하지 않을 때와 비밀번호가 틀렸을 때 에러메시지를 통일시키신 것을 보니 윤샘이 보안도 신경써주신 것 같다고 생각했습니다.
저는 사용자 검증 로직을 Spring Security의 AuthenticationManager.authenticate()에게 위임하는 방식으로 구현했는데, 이 방식을 이용하면 사용자를 검증하는 과정에서 Spring Security가 제공하는 보안 기능을 활용할 수 있다는 장점이 있더라고요! 특히 사용자가 존재하지 않을 때도 내부적으로 비밀번호 비교 연산을 진행해 타이밍 공격을 막을 수 있다고 합니다. (user enumeration이라구 하더라고용)
그리고 UserDetails에서 관리되는 계정 상태도 Spring Security에서 검증하기 때문에 나중에 사용자 계정 잠금이나 비활성화 같은 정책이 추가됐을 때에도 더 유지보수가 편리해진다는 장점도 있습니다!
물론 윤샘처럼 구현하는 것도 가독성 측면에서의 장점도 있는 것 같습니다. 다만 이미 Spring Security를 도입하신 만큼 기능을 활용하는 것도 고려해보시면 좋을 것 같습니다!! 😊
(아 그리고 로그인 요청 dto에 필드값 오타가 있습니다 ㅎㅎ)
| @RestController | ||
| @RequiredArgsConstructor | ||
| @RequestMapping("/public") | ||
| public class PublicMemberController { | ||
|
|
||
| private final MemberService memberService; |
There was a problem hiding this comment.
윤샘은 인증 관련 로직을 PublicMemberController로 분리하셨네요! 클래스명을 보니까 인증 관련 로직이 아니더라도 인증이 필요없는 API들을 분리하려고 의도하신 것 같습니다. Member도메인 특성상 필요한 API가 많다보니 유지보수 측면에서도 윤샘처럼 클래스를 분리하는 것이 더 낫다고 생각합니다.
하지만 지금 역할 분리가 컨트롤러 계층에서만 되어있고, 서비스 계층은 MemberService에 Public, Private API가 모두 통합되어있는 것 같습니다! 이렇게 되면 분리의 의미가 약해지고 구조를 파악하는 데 헷갈릴 수 있겠다는 생각이 듭니다.
서비스 계층은 따로 분리하지 않으신 윤샘의 이유가 궁금합니다!
| kakao: | ||
| client-id: ${KAKAO_REST_API_KEY} | ||
| client-secret: ${KAKAO_REST_API_SECRET} | ||
| client-authentication-method: client_secret_post |
4 participants
✅ 실습 체크리스트
✅ 컨벤션 체크리스트
📌 주안점